Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(theme): add mui v4 and v5 grid examples #187

Conversation

christoph-jerolimov
Copy link
Member

@christoph-jerolimov christoph-jerolimov commented Dec 14, 2024

Hey, I just made a Pull Request!

This PR adds

  1. some new Grid examples to test plugins to see and understand the difference between Backstage and RHDH. But there is none; both look except different colors and borders really similar!
  2. add workaround to load mui v5 theme correctly when package test plugins

Backstage defines a default spacing for all Grids, see:

https://github.com/backstage/backstage/blob/3d2e95f71c2be9a63530697cf01e482e4f50f98d/packages/theme/src/v5/defaultComponentThemes.ts#L67-L71

The RHDH theme doesn't change the padding or default spacing. But in MUI 5 it's required to add additional padding via spacing to the Grid container or to the content (in the Grid item).

The UI will looks unaligned when a padding is added to the Grid item.

MUI v4 and v5 looks similar if no padding is added to the Grid item

image

image

But it's not okay to add padding to Grid item in MUI v5:

MUI v4

image

MUI v5

image

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Dec 14, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-bc-test workspaces/theme/plugins/bc-test none v0.1.0
@red-hat-developer-hub/backstage-plugin-mui4-test workspaces/theme/plugins/mui4-test none v0.1.0
@red-hat-developer-hub/backstage-plugin-mui5-test workspaces/theme/plugins/mui5-test none v0.1.0

Copy link
Member

@debsmita1 debsmita1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the year in the notice to 2025 ?
otherwise, it looks good to me.

@christoph-jerolimov
Copy link
Member Author

Could you update the year in the notice to 2025 ? otherwise, it looks good to me.

@debsmita1 but the code is from 2024. :) Kashish created also #235 for this, so I like to keep it for now and update it later if needed after the other PR is merged (or this PR will update this headers as well if needed). But my understanding is that the year is when the code is written and shouldn't be updated to the current year.

@debsmita1 debsmita1 self-requested a review January 5, 2025 06:43
Copy link
Member

@debsmita1 debsmita1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 5, 2025
@christoph-jerolimov christoph-jerolimov merged commit 9e10c50 into redhat-developer:main Jan 5, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants